Use `tcx.short_string()` in more diagnostics #144039

estebank · 2025-07-16T19:45:42Z

TyCtxt::short_string ensures that user visible type paths aren't overwhelming on the terminal output, and properly saves the long name to disk as a side-channel. We already use these throughout the compiler and have been using them as needed when users find cases where the output is verbose. This is a proactive search of some cases to use short_string.

We add support for shortening the path of "trait path only".

Every manual use of short_string is a bright marker that that error should be using structured diagnostics instead (as they have proper handling of long types without the maintainer having to think abou tthem).

rustbot · 2025-07-16T19:45:46Z

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2025-07-16T19:45:48Z

HIR ty lowering was modified

cc @fmease

estebank · 2025-07-16T19:47:43Z

tests/ui/confuse-field-and-method/issue-2392.rs

+// FIXME(estebank): diagnostics with long type paths that don't print out the full path anywhere
+// still prints the note explaining where the type was written to.
+//@ compile-flags: -Zwrite-long-types-to-disk=yes


I found these by changing the default -Zwrite-long-types-to-disk=no in compiletest::runtest::TestCx::make_compile_args to yes, which highlighted some preexisting cases that I'll investigate.

One left: tests/ui/methods/filter-relevant-fn-bounds.rs. Also pre-existing.

compiler/rustc_hir_typeck/src/method/suggest.rs

compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs

fee1-dead · 2025-07-21T16:00:45Z

compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs

-                    let ty_str = tcx.short_string(ty, err.long_ty_path());
-                    let msg = format!("required because it appears within the type `{ty_str}`");
+                    let mut msg = || {
+                        let ty_str = tcx.short_string(ty, err.long_ty_path());
+                        format!("required because it appears within the type `{ty_str}`")
+                    };


This probably illustrates a problem we have with the current system: we don't prevent if a short string was created but was never printed. I think there could be a mechanism (a drop guard for a custom short_string return newtype) in which we detect that. Could you open an issue?

fee1-dead · 2025-07-21T16:05:49Z

tests/ui/confuse-field-and-method/issue-18343.stderr

These ui tests are now regressions because we should not "show generic arguments when the method can't be found in any implementations". If you're trying to prevent us from creating a short string when we don't use it, maybe we should pass a closure down instead of the ty_str_reported.

I think that I originally made the message only mention the item name instead of the Ty<'_> was precisely because we didn't have short_string back then and we could end up with quite long output accidentally. I was concerned back then that there are cases where the method is available on a given type depending on bounds on its type parameters, so I felt comfortable relying exclusively on short_string for keeping the output readable.

#81576 says we shouldn't show generics at all if they are irrelevant. I agree. I think we should spend effort keeping that behavior here.

Would it be ok to display the type's identity (Option<T> instead of the current Option and this PR's Option<ElaboratedType>) or a freshened type (Option<_>)? I'd really want to keep the structured error's field a Ty<'_> instead of a String, and using the identity allows for it.

fee1-dead · 2025-07-21T16:09:52Z

tests/ui/confuse-field-and-method/issue-2392.rs

inconsistent output in aarch64

seems quite odd. Could you explain what the issue is?

It was the error from #144039 (comment)

I suspect that that platform in particular prints a slightly different output for the closure type which puts it under the default terminal width threshold.

that platform in particular prints a slightly different output for the closure type

That is really really surprising for me. Could you post an issue for this? This seems very worthy of investigation.

`TyCtxt::short_string` ensures that user visible type paths aren't overwhelming on the terminal output, and properly saves the long name to disk as a side-channel. We already use these throughout the compiler and have been using them as needed when users find cases where the output is verbose. This is a proactive search of some cases to use `short_string`. We add support for shortening the path of "trait path only". Every manual use of `short_string` is a bright marker that that error should be using structured diagnostics instead (as they have proper handling of long types without the maintainer having to think abou tthem).

When we don't actually print out a shortened type we don't need the "use `--verbose`" note.

This test seems to have improper use of `TyCtxt::short_string`.

fee1-dead · 2025-07-24T09:29:48Z

compiler/rustc_hir_typeck/src/method/suggest.rs

+        &self,
+        rcvr_ty: Ty<'tcx>,
+        rcvr_expr: &hir::Expr<'tcx>,
+        mut file: Option<PathBuf>,


rename this to something more descriptive, e.g. long_ty_file

fee1-dead · 2025-07-24T09:34:06Z

tests/ui/confuse-field-and-method/issue-18343.stderr

#81576 says we shouldn't show generics at all if they are irrelevant. I agree. I think we should spend effort keeping that behavior here.

rustbot assigned fee1-dead Jul 16, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 16, 2025

estebank commented Jul 16, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

estebank force-pushed the short-paths branch from 38a3323 to dc45f5f Compare July 18, 2025 16:59

estebank commented Jul 18, 2025

View reviewed changes

compiler/rustc_hir_typeck/src/method/suggest.rs Show resolved Hide resolved

fee1-dead requested changes Jul 21, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 21, 2025

estebank added 7 commits July 21, 2025 20:13

Remove useless note

33b717d

When we don't actually print out a shortened type we don't need the "use `--verbose`" note.

Tweak use of short_string in method errors

a79998a

Mark test to use short type paths

e3da3b2

This test seems to have improper use of `TyCtxt::short_string`.

Minor fixes of short_string handling

1318324

Revert change to one test because of inconsistent output in aarc64

be6edc3

review comment: rename err_msg closure to default_err_msg

0786a11

estebank force-pushed the short-paths branch from dc45f5f to 0786a11 Compare July 21, 2025 20:16

estebank mentioned this pull request Jul 23, 2025

Devise mechanism for ensuring tcx.short_string is actually used #144374

Open

estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 23, 2025

fee1-dead reviewed Jul 24, 2025

View reviewed changes

Use tcx.short_string() in more diagnostics #144039

Are you sure you want to change the base?

Use tcx.short_string() in more diagnostics #144039

Conversation

estebank commented Jul 16, 2025

Uh oh!

rustbot commented Jul 16, 2025

Uh oh!

rustbot commented Jul 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Use `tcx.short_string()` in more diagnostics #144039

Use `tcx.short_string()` in more diagnostics #144039